[ECO-4940] NS_SWIFT_SENDABLE to every type that has a docstring
#1963
Conversation
WalkthroughThe updates introduce the Changes
Assessment against linked issues
Tip We have updated our review workflow to use the Anthropic's Claude family of models. Please share any feedback in the discussion post on our Discord. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
temporarily deployed
to
staging/pull/1963/features
github-actions
bot
temporarily deployed
to
staging/pull/1963/jazzydoc
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (43)
- Source/PrivateHeaders/Ably/ARTAttachRequestParams.h (1 hunks)
- Source/PrivateHeaders/Ably/ARTChannelStateChangeParams.h (1 hunks)
- Source/PrivateHeaders/Ably/ARTConnectionStateChangeParams.h (1 hunks)
- Source/PrivateHeaders/Ably/ARTTestClientOptions.h (1 hunks)
- Source/include/Ably/ARTAuth.h (1 hunks)
- Source/include/Ably/ARTAuthDetails.h (1 hunks)
- Source/include/Ably/ARTAuthOptions.h (1 hunks)
- Source/include/Ably/ARTChannel.h (1 hunks)
- Source/include/Ably/ARTChannelOptions.h (1 hunks)
- Source/include/Ably/ARTChannels.h (1 hunks)
- Source/include/Ably/ARTClientOptions.h (1 hunks)
- Source/include/Ably/ARTConnection.h (1 hunks)
- Source/include/Ably/ARTConnectionDetails.h (1 hunks)
- Source/include/Ably/ARTDataQuery.h (3 hunks)
- Source/include/Ably/ARTDeviceDetails.h (1 hunks)
- Source/include/Ably/ARTDeviceIdentityTokenDetails.h (1 hunks)
- Source/include/Ably/ARTDevicePushDetails.h (1 hunks)
- Source/include/Ably/ARTEventEmitter.h (1 hunks)
- Source/include/Ably/ARTHTTPPaginatedResponse.h (1 hunks)
- Source/include/Ably/ARTLocalDevice.h (1 hunks)
- Source/include/Ably/ARTMessage.h (1 hunks)
- Source/include/Ably/ARTPaginatedResult.h (1 hunks)
- Source/include/Ably/ARTPresence.h (1 hunks)
- Source/include/Ably/ARTPresenceMessage.h (2 hunks)
- Source/include/Ably/ARTPush.h (1 hunks)
- Source/include/Ably/ARTPushAdmin.h (1 hunks)
- Source/include/Ably/ARTPushChannel.h (1 hunks)
- Source/include/Ably/ARTPushChannelSubscription.h (1 hunks)
- Source/include/Ably/ARTPushChannelSubscriptions.h (1 hunks)
- Source/include/Ably/ARTPushDeviceRegistrations.h (1 hunks)
- Source/include/Ably/ARTRealtime.h (1 hunks)
- Source/include/Ably/ARTRealtimeChannel.h (1 hunks)
- Source/include/Ably/ARTRealtimeChannelOptions.h (2 hunks)
- Source/include/Ably/ARTRealtimePresence.h (2 hunks)
- Source/include/Ably/ARTRest.h (1 hunks)
- Source/include/Ably/ARTRestChannel.h (1 hunks)
- Source/include/Ably/ARTRestPresence.h (2 hunks)
- Source/include/Ably/ARTStats.h (10 hunks)
- Source/include/Ably/ARTStatus.h (3 hunks)
- Source/include/Ably/ARTTokenDetails.h (1 hunks)
- Source/include/Ably/ARTTokenParams.h (1 hunks)
- Source/include/Ably/ARTTokenRequest.h (1 hunks)
- Source/include/Ably/ARTTypes.h (14 hunks)
Additional comments not posted (72)
Source/include/Ably/ARTPresence.h (1)
10-10: NS_SWIFT_SENDABLE annotation added to ARTPresence.The addition of the
NS_SWIFT_SENDABLEannotation enhances concurrency support for Swift users by indicating that instances ofARTPresencecan be safely used across concurrency domains.Source/include/Ably/ARTAuthDetails.h (1)
8-8: NS_SWIFT_SENDABLE annotation added to ARTAuthDetails.The addition of the
NS_SWIFT_SENDABLEannotation enhances concurrency support for Swift users by indicating that instances ofARTAuthDetailscan be safely used across concurrency domains.Source/include/Ably/ARTLocalDevice.h (1)
11-11: NS_SWIFT_SENDABLE annotation added to ARTLocalDevice.The addition of the
NS_SWIFT_SENDABLEannotation enhances concurrency support for Swift users by indicating that instances ofARTLocalDevicecan be safely used across concurrency domains.Source/include/Ably/ARTDevicePushDetails.h (1)
10-10: NS_SWIFT_SENDABLE annotation added successfully.The addition of
NS_SWIFT_SENDABLEtoARTDevicePushDetailsenhances its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.Source/include/Ably/ARTChannelOptions.h (1)
11-11: NS_SWIFT_SENDABLE annotation added successfully.The addition of
NS_SWIFT_SENDABLEtoARTChannelOptionsenhances its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.Source/include/Ably/ARTDeviceIdentityTokenDetails.h (1)
8-8: NS_SWIFT_SENDABLE annotation added successfully.The addition of
NS_SWIFT_SENDABLEtoARTDeviceIdentityTokenDetailsenhances its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.Source/PrivateHeaders/Ably/ARTConnectionStateChangeParams.h (1)
13-13: Approved: Addition ofNS_SWIFT_SENDABLE.The addition of the
NS_SWIFT_SENDABLEattribute to theARTConnectionStateChangeParamsinterface is appropriate and aligns with the PR objectives to enhance Swift concurrency compatibility.Source/include/Ably/ARTRealtimeChannelOptions.h (1)
8-8: Approved: Addition ofNS_SWIFT_SENDABLE.The addition of the
NS_SWIFT_SENDABLEattribute to bothARTChannelModeandARTRealtimeChannelOptionsis appropriate and aligns with the PR objectives to enhance Swift concurrency compatibility.Also applies to: 33-33
Source/include/Ably/ARTDeviceDetails.h (1)
11-11: Approved: Addition ofNS_SWIFT_SENDABLE.The addition of the
NS_SWIFT_SENDABLEattribute to theARTDeviceDetailsinterface is appropriate and aligns with the PR objectives to enhance Swift concurrency compatibility.Source/include/Ably/ARTPushAdmin.h (1)
30-30: NS_SWIFT_SENDABLE annotation added successfully.The addition of
NS_SWIFT_SENDABLEto theARTPushAdmininterface enhances its compatibility with Swift's concurrency model. This change is appropriate for ensuring thread safety and interoperability.Source/include/Ably/ARTPushChannelSubscription.h (1)
8-8: NS_SWIFT_SENDABLE annotation added successfully.The addition of
NS_SWIFT_SENDABLEto theARTPushChannelSubscriptioninterface enhances its compatibility with Swift's concurrency model. This change is appropriate for ensuring thread safety and interoperability.Source/PrivateHeaders/Ably/ARTAttachRequestParams.h (1)
11-11: NS_SWIFT_SENDABLE annotation added successfully.The addition of
NS_SWIFT_SENDABLEto theARTAttachRequestParamsinterface enhances its compatibility with Swift's concurrency model. This change is appropriate for ensuring thread safety and interoperability.Source/PrivateHeaders/Ably/ARTTestClientOptions.h (1)
13-13: NS_SWIFT_SENDABLE annotation added successfully.The addition of the
NS_SWIFT_SENDABLEannotation to theARTTestClientOptionsinterface enhances Swift concurrency compatibility. This change is aligned with the PR objectives and does not affect existing functionality.Source/include/Ably/ARTChannels.h (1)
10-10: NS_SWIFT_SENDABLE annotation added successfully.The addition of the
NS_SWIFT_SENDABLEannotation to theARTChannelsinterface enhances Swift concurrency compatibility. This change is aligned with the PR objectives and does not affect existing functionality.Source/include/Ably/ARTHTTPPaginatedResponse.h (1)
10-10: NS_SWIFT_SENDABLE annotation added successfully.The addition of the
NS_SWIFT_SENDABLEannotation to theARTHTTPPaginatedResponseinterface enhances Swift concurrency compatibility. This change is aligned with the PR objectives and does not affect existing functionality.Source/include/Ably/ARTDataQuery.h (1)
8-8: NS_SWIFT_SENDABLE annotation added.The addition of
NS_SWIFT_SENDABLEtoARTQueryDirection,ARTDataQuery, andARTRealtimeHistoryQueryenhances Swift concurrency compatibility.Also applies to: 17-17, 45-45
Source/include/Ably/ARTPaginatedResult.h (1)
11-11: NS_SWIFT_SENDABLE annotation added.The addition of
NS_SWIFT_SENDABLEtoARTPaginatedResultenhances Swift concurrency compatibility.Source/include/Ably/ARTRestChannel.h (1)
49-49: NS_SWIFT_SENDABLE annotation added.The addition of
NS_SWIFT_SENDABLEtoARTRestChannelenhances Swift concurrency compatibility.Source/PrivateHeaders/Ably/ARTChannelStateChangeParams.h (1)
14-14: NS_SWIFT_SENDABLE Annotation Added.The addition of
NS_SWIFT_SENDABLEto theARTChannelStateChangeParamsinterface enhances its compatibility with Swift's concurrency model. This change aligns with the PR objectives and does not affect existing functionality.Source/include/Ably/ARTMessage.h (1)
12-12: NS_SWIFT_SENDABLE Annotation Added.The addition of
NS_SWIFT_SENDABLEto theARTMessageinterface enhances its compatibility with Swift's concurrency model. This change aligns with the PR objectives and does not affect existing functionality.Source/include/Ably/ARTPresenceMessage.h (2)
7-7: NS_SWIFT_SENDABLE Annotation Added to Enumeration.The addition of
NS_SWIFT_SENDABLEto theARTPresenceActionenumeration enhances its compatibility with Swift's concurrency model. This change aligns with the PR objectives and does not affect existing functionality.
39-39: NS_SWIFT_SENDABLE Annotation Added to Interface.The addition of
NS_SWIFT_SENDABLEto theARTPresenceMessageinterface enhances its compatibility with Swift's concurrency model. This change aligns with the PR objectives and does not affect existing functionality.Source/include/Ably/ARTPushDeviceRegistrations.h (1)
64-64: Addition ofNS_SWIFT_SENDABLEannotation is appropriate.The addition of the
NS_SWIFT_SENDABLEannotation to theARTPushDeviceRegistrationsinterface aligns with the PR objectives of enhancing Swift concurrency compatibility. This change does not affect existing functionality and is a positive step towards ensuring thread safety.Source/include/Ably/ARTTokenParams.h (1)
13-13: Addition ofNS_SWIFT_SENDABLEannotation is appropriate.The addition of the
NS_SWIFT_SENDABLEannotation to theARTTokenParamsinterface aligns with the PR objectives of enhancing Swift concurrency compatibility. This change does not affect existing functionality and is a positive step towards ensuring thread safety.Source/include/Ably/ARTPushChannelSubscriptions.h (1)
65-65: Addition ofNS_SWIFT_SENDABLEannotation is appropriate.The addition of the
NS_SWIFT_SENDABLEannotation to theARTPushChannelSubscriptionsinterface aligns with the PR objectives of enhancing Swift concurrency compatibility. This change does not affect existing functionality and is a positive step towards ensuring thread safety.Source/include/Ably/ARTPushChannel.h (1)
86-86: NS_SWIFT_SENDABLE Annotation AddedThe addition of the
NS_SWIFT_SENDABLEannotation to theARTPushChannelinterface is appropriate and enhances compatibility with Swift's concurrency model. No further changes are necessary.Source/include/Ably/ARTTokenDetails.h (1)
11-11: NS_SWIFT_SENDABLE Annotation AddedThe addition of the
NS_SWIFT_SENDABLEannotation to theARTTokenDetailsinterface is appropriate and enhances compatibility with Swift's concurrency model. No further changes are necessary.Source/include/Ably/ARTConnectionDetails.h (1)
8-8: NS_SWIFT_SENDABLE Annotation AddedThe addition of the
NS_SWIFT_SENDABLEannotation to theARTConnectionDetailsinterface is appropriate and enhances compatibility with Swift's concurrency model. No further changes are necessary.Source/include/Ably/ARTRestPresence.h (2)
13-13: NS_SWIFT_SENDABLE annotation added to ARTPresenceQuery.The addition of
NS_SWIFT_SENDABLEtoARTPresenceQueryis appropriate for enhancing Swift concurrency compatibility.
82-82: NS_SWIFT_SENDABLE annotation added to ARTRestPresence.The addition of
NS_SWIFT_SENDABLEtoARTRestPresenceis appropriate for enhancing Swift concurrency compatibility.Source/include/Ably/ARTTokenRequest.h (1)
12-12: NS_SWIFT_SENDABLE annotation added to ARTTokenRequest.The addition of
NS_SWIFT_SENDABLEtoARTTokenRequestis appropriate for enhancing Swift concurrency compatibility.Source/include/Ably/ARTChannel.h (1)
82-82: NS_SWIFT_SENDABLE annotation added to ARTChannel.The addition of
NS_SWIFT_SENDABLEtoARTChannelis appropriate for enhancing Swift concurrency compatibility.Source/include/Ably/ARTConnection.h (1)
83-83: NS_SWIFT_SENDABLE annotation added.The addition of the
NS_SWIFT_SENDABLEannotation to theARTConnectioninterface is appropriate for enhancing Swift concurrency compatibility. No issues found.Source/include/Ably/ARTRealtime.h (1)
107-107: NS_SWIFT_SENDABLE annotation added.The addition of the
NS_SWIFT_SENDABLEannotation to theARTRealtimeinterface is appropriate for enhancing Swift concurrency compatibility. No issues found.Source/include/Ably/ARTRest.h (1)
102-102: NS_SWIFT_SENDABLE annotation added.The addition of the
NS_SWIFT_SENDABLEannotation to theARTRestinterface is appropriate for enhancing Swift concurrency compatibility. No issues found.Source/include/Ably/ARTPush.h (1)
114-114: NS_SWIFT_SENDABLE Annotation Added to ARTPushThe addition of the
NS_SWIFT_SENDABLEannotation to theARTPushinterface is appropriate and aligns with the objective of enhancing interoperability with Swift's concurrency model. This change ensures that instances ofARTPushcan be safely used across Swift concurrency domains.Source/include/Ably/ARTAuth.h (1)
91-91: NS_SWIFT_SENDABLE Annotation Added to ARTAuthThe addition of the
NS_SWIFT_SENDABLEannotation to theARTAuthinterface is appropriate and aligns with the objective of enhancing interoperability with Swift's concurrency model. This change ensures that instances ofARTAuthcan be safely used across Swift concurrency domains.Source/include/Ably/ARTAuthOptions.h (1)
21-21: NS_SWIFT_SENDABLE Annotation Added to ARTAuthOptionsThe addition of the
NS_SWIFT_SENDABLEannotation to theARTAuthOptionsinterface is appropriate and aligns with the objective of enhancing interoperability with Swift's concurrency model. This change ensures that instances ofARTAuthOptionscan be safely used across Swift concurrency domains.Source/include/Ably/ARTEventEmitter.h (2)
26-26: NS_SWIFT_SENDABLE annotation added to ARTEventListener.The addition of
NS_SWIFT_SENDABLEtoARTEventListeneris appropriate for Swift concurrency compatibility.
33-33: NS_SWIFT_SENDABLE annotation added to ARTEventEmitter.The addition of
NS_SWIFT_SENDABLEtoARTEventEmitteris appropriate for Swift concurrency compatibility.Source/include/Ably/ARTRealtimeChannel.h (1)
152-152: NS_SWIFT_SENDABLE annotation added to ARTRealtimeChannel.The addition of
NS_SWIFT_SENDABLEtoARTRealtimeChannelis appropriate for Swift concurrency compatibility.Source/include/Ably/ARTClientOptions.h (1)
15-15: NS_SWIFT_SENDABLE annotation added to ARTClientOptions.The addition of
NS_SWIFT_SENDABLEtoARTClientOptionsis appropriate for Swift concurrency compatibility.Source/include/Ably/ARTStatus.h (3)
30-30: Addition ofNS_SWIFT_SENDABLEtoARTErrorCode.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTErrorCodeenumeration, enhancing its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.
193-193: Addition ofNS_SWIFT_SENDABLEtoARTErrorInfo.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTErrorInfointerface. This enhances its usability in Swift concurrency contexts and aligns with the PR objectives.
262-262: Addition ofNS_SWIFT_SENDABLEtoARTStatus.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatusinterface. This change improves its integration with Swift's concurrency model and is consistent with the PR objectives.Source/include/Ably/ARTRealtimePresence.h (2)
13-13: Addition ofNS_SWIFT_SENDABLEtoARTRealtimePresenceQuery.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTRealtimePresenceQueryinterface, enhancing its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.
221-221: Addition ofNS_SWIFT_SENDABLEtoARTRealtimePresence.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTRealtimePresenceinterface. This enhances its usability in Swift concurrency contexts and aligns with the PR objectives.Source/include/Ably/ARTStats.h (10)
10-10: Addition ofNS_SWIFT_SENDABLEtoARTStatsGranularity.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatsGranularityenumeration, enhancing its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.
33-33: Addition ofNS_SWIFT_SENDABLEtoARTStatsQuery.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatsQueryinterface, enhancing its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.
46-46: Addition ofNS_SWIFT_SENDABLEtoARTStatsMessageCount.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatsMessageCountinterface. This enhances its usability in Swift concurrency contexts and aligns with the PR objectives.
74-74: Addition ofNS_SWIFT_SENDABLEtoARTStatsMessageTypes.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatsMessageTypesinterface, enhancing its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.
108-108: Addition ofNS_SWIFT_SENDABLEtoARTStatsMessageTraffic.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatsMessageTrafficinterface. This enhances its usability in Swift concurrency contexts and aligns with the PR objectives.
148-148: Addition ofNS_SWIFT_SENDABLEtoARTStatsResourceCount.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatsResourceCountinterface, enhancing its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.
194-194: Addition ofNS_SWIFT_SENDABLEtoARTStatsConnectionTypes.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatsConnectionTypesinterface. This enhances its usability in Swift concurrency contexts and aligns with the PR objectives.
228-228: Addition ofNS_SWIFT_SENDABLEtoARTStatsRequestCount.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatsRequestCountinterface, enhancing its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.
262-262: Addition ofNS_SWIFT_SENDABLEtoARTStatsPushCount.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatsPushCountinterface. This enhances its usability in Swift concurrency contexts and aligns with the PR objectives.
314-314: Addition ofNS_SWIFT_SENDABLEtoARTStats.The
NS_SWIFT_SENDABLEannotation has been correctly added to theARTStatsinterface, enhancing its compatibility with Swift's concurrency model. This change is appropriate and aligns with the PR objectives.Source/include/Ably/ARTTypes.h (15)
37-37: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTAuthenticationis appropriate.
47-47: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTAuthMethodis appropriate.
56-56: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTRealtimeConnectionStateis appropriate.
98-98: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTRealtimeConnectionEventis appropriate.
120-120: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTRealtimeChannelStateis appropriate.
158-158: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTChannelEventis appropriate.
178-178: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTDataQueryErroris appropriate.
188-188: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTRealtimeHistoryErroris appropriate.
194-194: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTCustomRequestErroris appropriate.
233-233: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTConnectionStateChangeis appropriate.
277-277: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTChannelStateChangeis appropriate.
323-323: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTChannelMetricsis appropriate.
369-369: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTChannelOccupancyis appropriate.
385-385: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTChannelStatusis appropriate.
406-406: LGTM!The addition of
NS_SWIFT_SENDABLEtoARTChannelDetailsis appropriate.
|
Please can you mention the resolved issue in the commit message? |
… their accompanied enums and params types (issue #1962).
maratal
force-pushed
the
fix/1962-swift-concurrency-annotations
branch
from
0ac6b31 to
de49b67
Compare
github-actions
bot
temporarily deployed
to
staging/pull/1963/features
Done.
Yeah, Xcode doesn't show real file path, so somehow those ended up there, sorry, fixed in de49b67 |
github-actions
bot
temporarily deployed
to
staging/pull/1963/jazzydoc
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (39)
- Source/include/Ably/ARTAuth.h (1 hunks)
- Source/include/Ably/ARTAuthDetails.h (1 hunks)
- Source/include/Ably/ARTAuthOptions.h (1 hunks)
- Source/include/Ably/ARTChannel.h (1 hunks)
- Source/include/Ably/ARTChannelOptions.h (1 hunks)
- Source/include/Ably/ARTChannels.h (1 hunks)
- Source/include/Ably/ARTClientOptions.h (1 hunks)
- Source/include/Ably/ARTConnection.h (1 hunks)
- Source/include/Ably/ARTConnectionDetails.h (1 hunks)
- Source/include/Ably/ARTDataQuery.h (3 hunks)
- Source/include/Ably/ARTDeviceDetails.h (1 hunks)
- Source/include/Ably/ARTDeviceIdentityTokenDetails.h (1 hunks)
- Source/include/Ably/ARTDevicePushDetails.h (1 hunks)
- Source/include/Ably/ARTEventEmitter.h (1 hunks)
- Source/include/Ably/ARTHTTPPaginatedResponse.h (1 hunks)
- Source/include/Ably/ARTLocalDevice.h (1 hunks)
- Source/include/Ably/ARTMessage.h (1 hunks)
- Source/include/Ably/ARTPaginatedResult.h (1 hunks)
- Source/include/Ably/ARTPresence.h (1 hunks)
- Source/include/Ably/ARTPresenceMessage.h (2 hunks)
- Source/include/Ably/ARTPush.h (1 hunks)
- Source/include/Ably/ARTPushAdmin.h (1 hunks)
- Source/include/Ably/ARTPushChannel.h (1 hunks)
- Source/include/Ably/ARTPushChannelSubscription.h (1 hunks)
- Source/include/Ably/ARTPushChannelSubscriptions.h (1 hunks)
- Source/include/Ably/ARTPushDeviceRegistrations.h (1 hunks)
- Source/include/Ably/ARTRealtime.h (1 hunks)
- Source/include/Ably/ARTRealtimeChannel.h (1 hunks)
- Source/include/Ably/ARTRealtimeChannelOptions.h (2 hunks)
- Source/include/Ably/ARTRealtimePresence.h (2 hunks)
- Source/include/Ably/ARTRest.h (1 hunks)
- Source/include/Ably/ARTRestChannel.h (1 hunks)
- Source/include/Ably/ARTRestPresence.h (2 hunks)
- Source/include/Ably/ARTStats.h (10 hunks)
- Source/include/Ably/ARTStatus.h (3 hunks)
- Source/include/Ably/ARTTokenDetails.h (1 hunks)
- Source/include/Ably/ARTTokenParams.h (1 hunks)
- Source/include/Ably/ARTTokenRequest.h (1 hunks)
- Source/include/Ably/ARTTypes.h (14 hunks)
Files skipped from review due to trivial changes (15)
- Source/include/Ably/ARTAuthDetails.h
- Source/include/Ably/ARTChannelOptions.h
- Source/include/Ably/ARTConnectionDetails.h
- Source/include/Ably/ARTDataQuery.h
- Source/include/Ably/ARTLocalDevice.h
- Source/include/Ably/ARTPaginatedResult.h
- Source/include/Ably/ARTPresence.h
- Source/include/Ably/ARTPresenceMessage.h
- Source/include/Ably/ARTPushAdmin.h
- Source/include/Ably/ARTRealtime.h
- Source/include/Ably/ARTRest.h
- Source/include/Ably/ARTRestChannel.h
- Source/include/Ably/ARTStatus.h
- Source/include/Ably/ARTTokenParams.h
- Source/include/Ably/ARTTypes.h
Files skipped from review as they are similar to previous changes (24)
- Source/include/Ably/ARTAuth.h
- Source/include/Ably/ARTAuthOptions.h
- Source/include/Ably/ARTChannel.h
- Source/include/Ably/ARTChannels.h
- Source/include/Ably/ARTClientOptions.h
- Source/include/Ably/ARTConnection.h
- Source/include/Ably/ARTDeviceDetails.h
- Source/include/Ably/ARTDeviceIdentityTokenDetails.h
- Source/include/Ably/ARTDevicePushDetails.h
- Source/include/Ably/ARTEventEmitter.h
- Source/include/Ably/ARTHTTPPaginatedResponse.h
- Source/include/Ably/ARTMessage.h
- Source/include/Ably/ARTPush.h
- Source/include/Ably/ARTPushChannel.h
- Source/include/Ably/ARTPushChannelSubscription.h
- Source/include/Ably/ARTPushChannelSubscriptions.h
- Source/include/Ably/ARTPushDeviceRegistrations.h
- Source/include/Ably/ARTRealtimeChannel.h
- Source/include/Ably/ARTRealtimeChannelOptions.h
- Source/include/Ably/ARTRealtimePresence.h
- Source/include/Ably/ARTRestPresence.h
- Source/include/Ably/ARTStats.h
- Source/include/Ably/ARTTokenDetails.h
- Source/include/Ably/ARTTokenRequest.h
…queue` and enums (they transformed into swift enums which are sendable).
github-actions
bot
temporarily deployed
to
staging/pull/1963/features
github-actions
bot
temporarily deployed
to
staging/pull/1963/jazzydoc
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- Source/include/Ably/ARTDataQuery.h (1 hunks)
- Source/include/Ably/ARTPresenceMessage.h (1 hunks)
- Source/include/Ably/ARTRealtimeChannelOptions.h (1 hunks)
- Source/include/Ably/ARTRealtimePresence.h (1 hunks)
- Source/include/Ably/ARTRestPresence.h (1 hunks)
- Source/include/Ably/ARTStats.h (1 hunks)
- Source/include/Ably/ARTStatus.h (1 hunks)
- Source/include/Ably/ARTTypes.h (8 hunks)
Files skipped from review due to trivial changes (3)
- Source/include/Ably/ARTDataQuery.h
- Source/include/Ably/ARTPresenceMessage.h
- Source/include/Ably/ARTStats.h
Files skipped from review as they are similar to previous changes (5)
- Source/include/Ably/ARTRealtimeChannelOptions.h
- Source/include/Ably/ARTRealtimePresence.h
- Source/include/Ably/ARTRestPresence.h
- Source/include/Ably/ARTStatus.h
- Source/include/Ably/ARTTypes.h
…perties and `ARTRestChannels` calls only methods of a sendable object).
github-actions
bot
temporarily deployed
to
staging/pull/1963/features
github-actions
bot
temporarily deployed
to
staging/pull/1963/jazzydoc
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- Source/include/Ably/ARTConnectionDetails.h (1 hunks)
- Source/include/Ably/ARTRestChannels.h (1 hunks)
Files skipped from review due to trivial changes (1)
- Source/include/Ably/ARTConnectionDetails.h
Additional comments not posted (1)
Source/include/Ably/ARTRestChannels.h (1)
19-19: Approved addition ofNS_SWIFT_SENDABLE.The addition of the
NS_SWIFT_SENDABLEannotation is a crucial enhancement for Swift concurrency compatibility. This change aligns well with the objectives outlined in the PR and the linked issue.
github-actions
bot
temporarily deployed
to
staging/pull/1963/features
github-actions
bot
temporarily deployed
to
staging/pull/1963/jazzydoc
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
Source/ARTLocalDevice.m (1)
2-2: The import statement references a non-existent file.The import of
ARTDeviceDetails+Private.hinARTLocalDevice.mis referencing a file that does not exist in the repository. This import is likely unnecessary and should be reviewed to prevent potential compilation errors. Please verify the necessity of this import and update it accordingly.
- File:
Source/ARTLocalDevice.m- Line: 2
Analysis chain
Verify the necessity and usage of the new import.
The import of
ARTDeviceDetails+Private.hhas been added. Please ensure that it is necessary for the functionality withinARTLocalDevice.mand that it does not introduce redundancy or conflicts with other imports.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `ARTDeviceDetails+Private.h` in `ARTLocalDevice.m`. # Test: Search for usage of members or methods from `ARTDeviceDetails+Private.h` in `ARTLocalDevice.m`. rg --type objc -A 5 $'ARTDeviceDetails' Source/ARTLocalDevice.mLength of output: 254
Script:
#!/bin/bash # Description: Search for specific methods or properties from `ARTDeviceDetails+Private.h` in `ARTLocalDevice.m`. # Find the methods or properties declared in `ARTDeviceDetails+Private.h`. rg --type objc --no-filename --no-heading '^\s*[-+]' Source/ARTDeviceDetails+Private.h > methods.txt # Search for these methods or properties in `ARTLocalDevice.m`. while IFS= read -r method; do echo "Searching for usage of: $method" rg --type objc "$method" Source/ARTLocalDevice.m done < methods.txtLength of output: 186
Source/ARTJsonLikeEncoder.m (1)
20-20: Remove the unnecessary import statement.The file
ARTDeviceDetails+Private.hdoes not exist in the repository, making the import statement inARTJsonLikeEncoder.munnecessary. Please remove this import to avoid potential compilation issues.
- File:
Source/ARTJsonLikeEncoder.m- Line:
#import "ARTDeviceDetails+Private.h"Analysis chain
Verify the necessity and usage of the new import.
The import of
ARTDeviceDetails+Private.hhas been added. Please ensure that it is necessary for the functionality withinARTJsonLikeEncoder.mand that it does not introduce redundancy or conflicts with other imports.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `ARTDeviceDetails+Private.h` in `ARTJsonLikeEncoder.m`. # Test: Search for usage of members or methods from `ARTDeviceDetails+Private.h` in `ARTJsonLikeEncoder.m`. rg --type objc -A 5 $'ARTDeviceDetails' Source/ARTJsonLikeEncoder.mLength of output: 2734
Script:
#!/bin/bash # Description: Check the contents of `ARTDeviceDetails+Private.h` and verify their usage in `ARTJsonLikeEncoder.m`. # Step 1: Display the contents of `ARTDeviceDetails+Private.h` to identify private members or methods. fd 'ARTDeviceDetails+Private.h' -x cat {} # Step 2: Search for usage of any identified private members or methods in `ARTJsonLikeEncoder.m`. # This step will be executed after reviewing the contents of the private header.Length of output: 43
Script:
#!/bin/bash # Description: Verify the existence of `ARTDeviceDetails+Private.h` and display its contents. # Step 1: Search for the file `ARTDeviceDetails+Private.h` in the repository. fd 'ARTDeviceDetails+Private.h' # Step 2: If found, display its contents. fd 'ARTDeviceDetails+Private.h' -x cat {}Length of output: 75
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- Ably.xcodeproj/project.pbxproj (10 hunks)
- Source/ARTChannel.m (2 hunks)
- Source/ARTChannelOptions.m (2 hunks)
- Source/ARTDeviceDetails.m (1 hunks)
- Source/ARTJsonLikeEncoder.m (1 hunks)
- Source/ARTLocalDevice.m (1 hunks)
- Source/ARTRealtimeChannelOptions.m (1 hunks)
- Source/Ably.modulemap (1 hunks)
- Source/PrivateHeaders/Ably/ARTChannelOptions+Private.h (1 hunks)
- Source/PrivateHeaders/Ably/ARTDeviceDetails+Private.h (1 hunks)
- Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h (1 hunks)
- Source/include/Ably/ARTDeviceDetails.h (1 hunks)
- Source/include/Ably/ARTLocalDevice.h (2 hunks)
- Test/Test Utilities/NSObject+TestSuite.h (1 hunks)
- Test/Test Utilities/NSObject+TestSuite.m (1 hunks)
- Test/Tests/RealtimeClientChannelTests.swift (1 hunks)
Files skipped from review due to trivial changes (1)
- Ably.xcodeproj/project.pbxproj
Additional comments not posted (17)
Source/PrivateHeaders/Ably/ARTChannelOptions+Private.h (1)
1-8: Review of New File: ARTChannelOptions+Private.hThe introduction of the
frozenproperty with a custom getter is a good practice for boolean properties in Objective-C, enhancing code readability. The file structure adheres to conventions for private headers, and necessary imports are included.The changes are approved.
Verification successful
Verification Successful: Proper Usage of
frozenPropertyThe
frozenproperty is used correctly in the codebase to enforce immutability by throwing exceptions when modifications are attempted after the options have been passed to a receiver. This aligns with the intended design and purpose of the property.
- Verified usage in
Source/ARTRealtimeChannelOptions.m- Verified usage in
Source/ARTChannelOptions.mScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `frozen` property in the SDK. # Test: Search for the usage of the `frozen` property. Expect: Proper usage in the SDK. rg --type objc -A 5 $'isFrozen'Length of output: 1847
Source/PrivateHeaders/Ably/ARTDeviceDetails+Private.h (1)
1-19: Review of New File: ARTDeviceDetails+Private.hThe introduction of several properties with
readwriteattributes in a private header is appropriate, allowing internal modifications. The use ofnullableandNSDictionarytypes is correctly applied, and the nullability macros are used properly.The changes are approved.
Source/include/Ably/ARTLocalDevice.h (1)
Line range hint
1-22: Review of Changes in ARTLocalDevice.hThe addition of the
NS_SWIFT_SENDABLEattribute to theARTLocalDeviceinterface is a significant enhancement for Swift concurrency compatibility. The file structure adheres to conventions for public headers, and necessary imports are included.The changes are approved.
Verification successful
NS_SWIFT_SENDABLE Attribute Usage Verified
TheNS_SWIFT_SENDABLEattribute is used consistently across the SDK, including in theARTLocalDeviceinterface, enhancing Swift concurrency compatibility as intended. The review comment is verified as correct.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `NS_SWIFT_SENDABLE` attribute in the SDK. # Test: Search for the usage of the `NS_SWIFT_SENDABLE` attribute. Expect: Proper usage in the SDK. rg --type objc -A 5 $'NS_SWIFT_SENDABLE'Length of output: 13583
Source/ARTChannelOptions.m (3)
6-6: Instance variable_cipheraddition approved.The addition of
_cipherenhances the class's functionality by allowing it to store cipher parameters.
20-22: Getter method for_cipherapproved.The method correctly returns the value of
_cipher.
24-31: Setter method for_cipherapproved with commendation for robust exception handling.The method correctly handles the
isFrozenstate by throwing an exception if an attempt is made to modify the cipher parameters after the object is frozen. This is crucial for maintaining object integrity in multi-threaded environments.Source/ARTRealtimeChannelOptions.m (3)
5-6: Instance variables_paramsand_modesaddition approved.The addition of
_paramsand_modesenhances the class's encapsulation by allowing it to store channel options securely.
9-11: Getter methods for_paramsand_modesapproved.The methods correctly return the values of
_paramsand_modes.Also applies to: 22-24
13-20: Setter methods for_paramsand_modesapproved with commendation for robust exception handling.The methods correctly handle the
isFrozenstate by throwing exceptions if an attempt is made to modify the parameters or modes after the object is frozen. This is crucial for maintaining object integrity in multi-threaded environments.Also applies to: 26-33
Source/include/Ably/ARTDeviceDetails.h (1)
16-16: Changes to properties approved.The changes to make the properties
id,clientId,platform,formFactor,metadata, andpushread-only are approved. These changes enhance encapsulation and promote immutability, aligning with best practices in object-oriented design.Also applies to: 21-21, 26-26, 31-31, 36-36, 41-41
Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h (1)
22-22: Approve the addition of thesecretproperty.The addition of the
secretproperty toARTLocalDeviceis a positive change for enhancing device-specific security features. Ensure that the integration of this property with existing functionalities is thoroughly tested.The code changes are approved.
Run the following script to verify the integration of the new property:
Verification successful
The
secretproperty is well-integrated with existing functionalities.The
secretproperty is being used in various parts of the codebase, including storage, retrieval, and network requests, indicating proper integration. Ensure thorough testing as suggested in the review comment to maintain security and functionality.
- Source/ARTLocalDevice.m: Setting and storing the
secret.- Source/ARTNSMutableRequest+ARTPush.m: Using the
secretfor device authentication.- Source/ARTJsonLikeEncoder.m: Including the
secretin dictionary representation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the `secret` property with existing functionalities. # Test: Search for the usage of the `secret` property. Expect: Proper handling and usage in the codebase. rg --type objc -A 5 $'secret'Length of output: 7124
Test/Test Utilities/NSObject+TestSuite.h (1)
34-34: Approve the addition of thetryInObjCfunction.The introduction of the
tryInObjCfunction is a valuable addition for structured error handling in Objective-C. Ensure that the function is correctly implemented and used throughout the codebase.The code changes are approved.
Run the following script to verify the correct implementation and usage of the
tryInObjCfunction:Source/ARTDeviceDetails.m (1)
3-3: Approve the import of theARTDeviceDetails+Private.hfile.The import of
ARTDeviceDetails+Private.his a positive change for improving modularity and maintainability of theARTDeviceDetailsclass. Ensure that the integration of this import with existing functionalities is thoroughly tested.The code changes are approved.
Run the following script to verify the integration of the new import:
Test/Test Utilities/NSObject+TestSuite.m (1)
52-60: Well-implemented error handling mechanism.The
tryInObjCfunction correctly encapsulates a non-escaping block within a try-catch structure, adhering to Objective-C's error handling conventions. This addition enhances the error handling capabilities of the SDK.The implementation is approved as it meets the requirements and follows best practices.
Source/Ably.modulemap (1)
39-39: Approval of header file additions with a documentation reminder.The inclusion of
ARTChannelOptions+Private.handARTDeviceDetails+Private.hin the module map is appropriate for enhancing SDK functionality. Ensure that these files are well-documented to clarify their purpose and usage.The changes are approved, but it's recommended to update the documentation accordingly.
Also applies to: 47-47
Source/ARTChannel.m (1)
3-3: Good practice enforced, but ensure compatibility.Setting
_options.frozen = YES;during initialization enforces immutability, which is generally a good practice to prevent unintended side effects. However, ensure that this change is thoroughly tested to verify that it does not adversely affect existing functionalities where options might have been modified post-initialization.The changes are approved, but thorough testing is recommended.
Also applies to: 26-26
Test/Tests/RealtimeClientChannelTests.swift (1)
4790-4826: Ensure immutability ofARTRealtimeChannelOptionsafter instantiation.The test method
test__140__ChannelOptions__options_provided_when_instantiating_a_channel_should_be_frozenis designed to verify that onceARTRealtimeChannelOptionsare used to instantiate a channel, they become immutable. This is confirmed by attempting to modify thecipher,modes, andparamsproperties ofchannelOptionsafter the channel has been instantiated, which correctly raisesNSExceptionName.objectInaccessibleException.The test method is correctly implemented and follows good testing practices by checking for exceptions when trying to modify frozen objects.
github-actions
bot
temporarily deployed
to
staging/pull/1963/features
github-actions
bot
temporarily deployed
to
staging/pull/1963/jazzydoc
github-actions
bot
temporarily deployed
to
staging/pull/1963/features
github-actions
bot
temporarily deployed
to
staging/pull/1963/jazzydoc
…etails`) as `readonly`. Redeclared them as `readwrite` in a private interface.
…ptions after channel initialization.
maratal
force-pushed
the
fix/1962-swift-concurrency-annotations
branch
from
eba2bfd to
58ccb88
Compare
github-actions
bot
temporarily deployed
to
staging/pull/1963/features
github-actions
bot
temporarily deployed
to
staging/pull/1963/jazzydoc
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
Source/ARTChannel.m (1)
26-26: Approved setting of_options.frozen.Setting
_options.frozentoYESduring initialization is a good practice to ensure immutability and thread safety. It would be beneficial to add a comment explaining why this change is necessary, especially for future maintainers or developers unfamiliar with the concurrency requirements.Consider adding a comment in the code to explain the purpose of setting
_options.frozentoYES, to enhance code readability and maintainability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- Ably.xcodeproj/project.pbxproj (10 hunks)
- Source/ARTAuth.m (1 hunks)
- Source/ARTChannel.m (2 hunks)
- Source/ARTChannelOptions.m (2 hunks)
- Source/ARTDeviceDetails.m (1 hunks)
- Source/ARTJsonLikeEncoder.m (1 hunks)
- Source/ARTLocalDevice.m (1 hunks)
- Source/ARTRealtimeChannelOptions.m (1 hunks)
- Source/Ably.modulemap (1 hunks)
- Source/PrivateHeaders/Ably/ARTChannelOptions+Private.h (1 hunks)
- Source/PrivateHeaders/Ably/ARTDeviceDetails+Private.h (1 hunks)
- Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h (1 hunks)
- Source/include/Ably/ARTDeviceDetails.h (1 hunks)
- Source/include/Ably/ARTLocalDevice.h (2 hunks)
- Test/Test Utilities/NSObject+TestSuite.h (1 hunks)
- Test/Test Utilities/NSObject+TestSuite.m (1 hunks)
- Test/Tests/RealtimeClientChannelTests.swift (2 hunks)
Files skipped from review due to trivial changes (6)
- Source/ARTAuth.m
- Source/ARTJsonLikeEncoder.m
- Source/ARTLocalDevice.m
- Source/Ably.modulemap
- Source/PrivateHeaders/Ably/ARTDeviceDetails+Private.h
- Source/include/Ably/ARTLocalDevice.h
Files skipped from review as they are similar to previous changes (10)
- Ably.xcodeproj/project.pbxproj
- Source/ARTChannelOptions.m
- Source/ARTDeviceDetails.m
- Source/ARTRealtimeChannelOptions.m
- Source/PrivateHeaders/Ably/ARTChannelOptions+Private.h
- Source/PrivateHeaders/Ably/ARTLocalDevice+Private.h
- Source/include/Ably/ARTDeviceDetails.h
- Test/Test Utilities/NSObject+TestSuite.h
- Test/Test Utilities/NSObject+TestSuite.m
- Test/Tests/RealtimeClientChannelTests.swift
Additional comments not posted (1)
Source/ARTChannel.m (1)
3-3: Approved import statement.The addition of
ARTChannelOptions+Private.his necessary for accessing private properties or methods that are likely used in the initialization logic.
|
LGTM once @lawrence-forooghian is happy with the open conversation |
|
Looks good, I just want to test it out in Chat to check that it makes our warnings go away, will do that shortly |
github-actions
bot
temporarily deployed
to
staging/pull/1963/features
Made a quick check and added private interface to |
github-actions
bot
temporarily deployed
to
staging/pull/1963/jazzydoc
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- Ably.xcodeproj/project.pbxproj (11 hunks)
- Source/ARTDevicePushDetails.m (1 hunks)
- Source/ARTJsonLikeEncoder.m (1 hunks)
- Source/Ably.modulemap (1 hunks)
- Source/PrivateHeaders/Ably/ARTDevicePushDetails+Private.h (1 hunks)
- Source/include/Ably/ARTDeviceDetails.h (1 hunks)
- Source/include/Ably/ARTDevicePushDetails.h (1 hunks)
- Source/include/Ably/ARTRealtimeChannels.h (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- Ably.xcodeproj/project.pbxproj
- Source/ARTJsonLikeEncoder.m
- Source/Ably.modulemap
Additional comments not posted (16)
Source/PrivateHeaders/Ably/ARTDevicePushDetails+Private.h (3)
1-2: LGTM!The code changes are approved.
4-4: LGTM!The code changes are approved.
8-14: LGTM!The code changes are approved.
Source/ARTDevicePushDetails.m (1)
2-3: Approve the changes to the import statements.The changes to the import statements, including the addition of the import for
ARTDevicePushDetails+Private.h, are approved.Verify that the use of private methods or properties does not introduce any unintended side effects or break the public API contract.
The use of a private category can enhance encapsulation and allow the class to access more specialized features or internal logic. However, it is important to ensure that the use of private methods or properties does not introduce any unintended side effects or break the public API contract.
Please confirm that the use of private methods or properties in
ARTDevicePushDetails+Private.hdoes not introduce any unintended side effects or break the public API contract.Source/include/Ably/ARTDevicePushDetails.h (4)
10-10: LGTM!The
NS_SWIFT_SENDABLEannotation is correctly added to theARTDevicePushDetailsinterface, indicating that instances can be safely used across Swift concurrency contexts. This aligns with the PR objective of enhancing compatibility with Swift's concurrency model.
16-16: LGTM!The
recipientproperty is correctly changed to read-only, which enforces encapsulation by preventing direct modification from outside the class. This is a good practice to manage mutability, especially since the property type isNSMutableDictionary.
21-21: LGTM!The
stateproperty is correctly changed to read-only, which enforces encapsulation by preventing direct modification from outside the class.
26-26: LGTM!The
errorReasonproperty is correctly changed to read-only, which enforces encapsulation by preventing direct modification from outside the class.Source/include/Ably/ARTRealtimeChannels.h (1)
20-20: LGTM!The addition of the
NS_SWIFT_SENDABLEannotation to theARTRealtimeChannelsinterface is a valuable change that enhances the usability and safety of the class in Swift applications that utilize structured concurrency features. This annotation indicates that instances of the class can be safely used across Swift concurrency domains, helping to prevent concurrency-related issues and providing clarity for Swift developers.The change does not alter the existing functionality of the class but adds a layer of safety and interoperability with Swift's concurrency model. This is a positive improvement that aligns with the objectives outlined in the linked issue and contributes to the overall robustness and usability of the SDK.
The code change is approved.
Source/include/Ably/ARTDeviceDetails.h (7)
11-11: LGTM!The
NS_SWIFT_SENDABLEannotation is correctly added to theARTDeviceDetailsclass, indicating that instances can be safely used across Swift concurrency domains. This change aligns with the PR objective of enhancing compatibility with Swift's concurrency model.
17-17: LGTM!Changing the access level of the
idproperty to read-only is a good practice. It enhances encapsulation by preventing external modification of the property after its initial assignment, thereby promoting immutability and potentially improving thread safety.
22-22: LGTM!Changing the access level of the
clientIdproperty to read-only is a good practice. It enhances encapsulation by preventing external modification of the property after its initial assignment, thereby promoting immutability and potentially improving thread safety.
27-27: LGTM!Changing the access level of the
platformproperty to read-only is a good practice. It enhances encapsulation by preventing external modification of the property after its initial assignment, thereby promoting immutability and potentially improving thread safety.
32-32: LGTM!Changing the access level of the
formFactorproperty to read-only is a good practice. It enhances encapsulation by preventing external modification of the property after its initial assignment, thereby promoting immutability and potentially improving thread safety.
37-37: LGTM!Changing the access level of the
metadataproperty to read-only is a good practice. It enhances encapsulation by preventing external modification of the property after its initial assignment, thereby promoting immutability and potentially improving thread safety.
42-42: LGTM!Changing the access level of the
pushproperty to read-only is a good practice. It enhances encapsulation by preventing external modification of the property after its initial assignment, thereby promoting immutability and potentially improving thread safety.
…iceDetails` and `ARTDevicePushDetails` sendable.
maratal
force-pushed
the
fix/1962-swift-concurrency-annotations
branch
from
7ae9700 to
dc8f168
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- Ably.xcodeproj/project.pbxproj (11 hunks)
- Source/ARTDevicePushDetails.m (1 hunks)
- Source/ARTJsonLikeEncoder.m (1 hunks)
- Source/Ably.modulemap (1 hunks)
- Source/PrivateHeaders/Ably/ARTDevicePushDetails+Private.h (1 hunks)
- Source/include/Ably/ARTDeviceDetails.h (1 hunks)
- Source/include/Ably/ARTDevicePushDetails.h (1 hunks)
Files skipped from review due to trivial changes (4)
- Ably.xcodeproj/project.pbxproj
- Source/ARTJsonLikeEncoder.m
- Source/Ably.modulemap
- Source/PrivateHeaders/Ably/ARTDevicePushDetails+Private.h
Files skipped from review as they are similar to previous changes (2)
- Source/ARTDevicePushDetails.m
- Source/include/Ably/ARTDevicePushDetails.h
Additional comments not posted (7)
Source/include/Ably/ARTDeviceDetails.h (7)
11-11: LGTM!The
NS_SWIFT_SENDABLEannotation is correctly added to theARTDeviceDetailsclass interface, indicating that instances of the class can be safely used across Swift concurrency contexts. This change aligns with the PR objective of enhancing the compatibility of the SDK with Swift's concurrency model.
17-17: LGTM!The access level of the
idproperty is correctly changed toreadonly, preventing external modification after its initial assignment. This change enhances encapsulation and promotes immutability, potentially improving thread safety.
22-22: LGTM!The access level of the
clientIdproperty is correctly changed toreadonly, preventing external modification after its initial assignment. This change enhances encapsulation and promotes immutability, potentially improving thread safety.
27-27: LGTM!The access level of the
platformproperty is correctly changed toreadonly, preventing external modification after its initial assignment. This change enhances encapsulation and promotes immutability, potentially improving thread safety.
32-32: LGTM!The access level of the
formFactorproperty is correctly changed toreadonly, preventing external modification after its initial assignment. This change enhances encapsulation and promotes immutability, potentially improving thread safety.
37-37: LGTM!The access level of the
metadataproperty is correctly changed toreadonly, preventing external modification after its initial assignment. This change enhances encapsulation and promotes immutability, potentially improving thread safety.
42-42: LGTM!The access level of the
pushproperty is correctly changed toreadonly, preventing external modification after its initial assignment. This change enhances encapsulation and promotes immutability, potentially improving thread safety.
|
Tested again in Chat and there are no warnings now. Still might be some types that Chat isn't using that aren't covered here, but I’m happy to approve and address other needs if we discover them. |
I've added
NS_SWIFT_SENDABLEto every type that has a docstring and their accompanied enums and params types.Closes #1962
Summary by CodeRabbit
New Features
NS_SWIFT_SENDABLEattribute to multiple interfaces, enhancing compatibility with Swift's concurrency model for safer multi-threaded use.Improvements
ARTDeviceDetailsandARTDevicePushDetailsto be read-only, enhancing encapsulation.ARTChannelOptions, reinforcing data integrity.tryInObjCfunction.ARTRealtimeChannelOptions, ensuring robust channel configuration.